Skip to content

Conversation

@fengming-ye
Copy link
Contributor

@fengming-ye fengming-ye commented Oct 16, 2024

Fix DPP build error when HOSTAPD enabled and DPP disabled.
Guard hapd_dpp_dispatch in both CONFIG_WIFI_NM_WPA_SUPPLICANT_DPP
and CONFIG_WIFI_NM_HOSTAPD_AP.

@fengming-ye
Copy link
Contributor Author

to fix issue #79897

@jukkar
Copy link
Member

jukkar commented Oct 16, 2024

Please add relevant information why the old commit needs to be reverted to the commit message.

@fengming-ye
Copy link
Contributor Author

Please add relevant information why the old commit needs to be reverted to the commit message.

updated

@jukkar
Copy link
Member

jukkar commented Oct 16, 2024

But why do we need to revert the commit 1da74ef, what kind of issue it is causing now?

@fengming-ye
Copy link
Contributor Author

But why do we need to revert the commit 1da74ef, what kind of issue it is causing now?

Pls check this bug #79897.
It causes build error when DPP disabled and HOSTAPD enabled, because of incomplete wrap.
So why do we need this commit 1da74ef? From my side it is conflict to review comments in the original DPP PR #73707.
@krish2718 any comments?

@jukkar
Copy link
Member

jukkar commented Oct 16, 2024

But wouldn't the original issue then be present what the 1da74ef is fixing if we revert it?

@fengming-ye
Copy link
Contributor Author

fengming-ye commented Oct 16, 2024

But wouldn't the original issue then be present what the 1da74ef is fixing if we revert it?

If hostap disables DPP (by disabling CONFIG_WIFI_NM_WPA_SUPPLICANT_DPP, which is zephyr only way to disable DPP),
L2 DPP code will be still there and can be tested, no build error, which is from original review comments request.
Only hostap will return fail when DPP L2 api is called, which won't be trouble.
So I don't see what 1da74ef is trying to fix. Could you pls explain it in detail?

@krish2718
Copy link
Contributor

But wouldn't the original issue then be present what the 1da74ef is fixing if we revert it?

If hostap disables DPP (by disabling CONFIG_WIFI_NM_WPA_SUPPLICANT_DPP, which is zephyr only way to disable DPP), L2 DPP code will be still there and can be tested, no build error, which is from original review comments request. Only hostap will return fail when DPP L2 api is called, which won't be trouble. So I don't see what 1da74ef is trying to fix. Could you pls explain it in detail?

Please see #79897 (comment) just including the header file is enough to get this error, but for some reason not seen with upstream.

Fix DPP build error when HOSTAPD enabled and DPP disabled.
Guard hapd_dpp_dispatch in both CONFIG_WIFI_NM_WPA_SUPPLICANT_DPP
and CONFIG_WIFI_NM_HOSTAPD_AP.

Signed-off-by: Fengming Ye <[email protected]>
@fengming-ye fengming-ye changed the title Revert "net: wifi: Fix DPP disabled build" modules: hostap: fix DPP build error Oct 16, 2024
@fengming-ye
Copy link
Contributor Author

@jukkar @krish2718 updated fix. thanks for clarification.

@jukkar jukkar added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Oct 16, 2024
@krish2718
Copy link
Contributor

Not sure about twister policies in upstream, but should we add these combinations to avoid such issues?

@jukkar
Copy link
Member

jukkar commented Oct 16, 2024

Not sure about twister policies in upstream, but should we add these combinations to avoid such issues?

That would be a good idea to add various config combinations to tests, not sure if tests/net/wifi/wifi_nm is a good place for those. Perhaps separate directory next to it to compile test various config combinations or enable-all type config.

@krish2718
Copy link
Contributor

Perhaps separate directory next to it to compile test various config combinations or enable-all type config

Just saw a PR merged with build_all that builds all drivers, we can have something similar for WPA supp?

@jukkar
Copy link
Member

jukkar commented Oct 17, 2024

Just saw a PR merged with build_all that builds all drivers, we can have something similar for WPA supp?

Yeah we could, I can prepare a PR for that.

@carlescufi carlescufi merged commit 9e8b7bd into zephyrproject-rtos:main Oct 17, 2024
27 checks passed
@fengming-ye fengming-ye deleted the fix/dpp_build_error branch February 6, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Networking area: Wi-Fi Wi-Fi Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants